-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use built-in is_terminal instead of is_terminal::is_terminal #9550
Conversation
oh, cool. so this does the same job as is_atty but stabelized in rust. nice. the only problem right now is that we're not on 1.70.0. we're on 1.68.2. 1 less crate is great and fits right in with our goals. i'll talk to the core-team to see if they think it's worth making the jump to 1.70 for this. |
Yes, they perform exactly the same function in *nix. https://github.com/softprops/atty/blob/7b5df17888997d57c2c1c8f91da1db5691f49953/src/lib.rs#L39-L49 |
Also worth noting that atty hasn't been maintained in a long while. The std version includes bug fixes (at least for Windows). There's also the |
@nibon7 I think we're going to wait on this. We want to do it but we probably need to maintain our 2-rust-versions-behind strategy so that we can have as broad a distribution as possible. Let's just keep this draft and revisit it again later. Good idea and Thanks!! |
Until we bump our minimal Rust version to `1.70.0` we can't use `std::io::IsTerminal`. The crate `is-terminal` (depending on `rustix` or `windows-sys`) can provide the same. Get's rid of the dependency on the outdated `atty` crate. We already transitively depend on it (e.g. through `miette`) As soon as we reach the new Rust version we can supersede this with @nibon7's nushell#9550 Co-authored-by: nibon7 <nibon7@163.com>
# Description Until we bump our minimal Rust version to `1.70.0` we can't use `std::io::IsTerminal`. The crate `is-terminal` (depending on `rustix` or `windows-sys`) can provide the same. Get's rid of the dependency on the outdated `atty` crate. We already transitively depend on it (e.g. through `miette`) As soon as we reach the new Rust version we can supersede this with @nibon7's #9550 Co-authored-by: nibon7 <nibon7@163.com>
Thanks for rebasing on it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for following up!
…#9550) # Description This PR tries to remove ~atty~ is-terminal from the entire code base, since ~[atty is unmaintained](https://rustsec.org/advisories/RUSTSEC-2021-0145) and~ [`is_terminal` has been stabilized](https://blog.rust-lang.org/2023/06/01/Rust-1.70.0.html#isterminal) in rust 1.70.0. cc @fdncred # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect -A clippy::result_large_err` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
Description
This PR tries to remove
attyis-terminal from the entire code base, sinceatty is unmaintained andis_terminal
has been stabilized in rust 1.70.0.cc @fdncred
User-Facing Changes
Tests + Formatting
After Submitting